-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Fix GH-12727: NumberFormatter constructor throws an exception on #12868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a5840e8
to
4fd8ae4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks sensible to me
ext/intl/formatter/formatter_main.c
Outdated
@@ -63,6 +64,11 @@ static int numfmt_ctor(INTERNAL_FUNCTION_PARAMETERS, zend_error_handling *error_ | |||
locale = intl_locale_get_default(); | |||
} | |||
|
|||
if (strlen(uloc_getISO3Language(locale)) == 0) { | |||
zend_argument_value_error(1, "is invalid"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be nice to show what the locale is to say that it is invalid. So maybe something like
zend_argument_value_error(1, "is invalid"); | |
zend_argument_value_error(1, " \"%s\" is invalid", locale); |
908d199
to
cfef555
Compare
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR, "datefmt_create: invalid locale", 0); | ||
return FAILURE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intl_error_set(NULL, U_ILLEGAL_ARGUMENT_ERROR, "datefmt_create: invalid locale", 0); | |
return FAILURE; | |
zend_argument_value_error(1, "\"%s\" is invalid", locale); | |
return FAILURE; |
Apply the OO error style to procedural?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep agreed
invalid locale. Also re-establishing exception throwing on IntlDateFormatter constructor overwritten by accident most likely so postponing it for next major release.
cfef555
to
a7b7bba
Compare
invalid locale.
Also re-establishing exception throwing on IntlDateFormatter constructor overwritten by accident most likely so postponing it for next major release.